Skip to content

refactor(forks/lstar): type SignedBlock.proof and rename multi-signature types#799

Merged
tcoratger merged 3 commits into
leanEthereum:mainfrom
tcoratger:refactor/signedblock-proof-typed
May 29, 2026
Merged

refactor(forks/lstar): type SignedBlock.proof and rename multi-signature types#799
tcoratger merged 3 commits into
leanEthereum:mainfrom
tcoratger:refactor/signedblock-proof-typed

Conversation

@tcoratger
Copy link
Copy Markdown
Collaborator

@tcoratger tcoratger commented May 29, 2026

Summary

Three related commits that clean up the block-level proof field:

  1. fdd0f947 — Type SignedBlock.proof as the typed envelope (later renamed to MultiMessageAggregate — see below). The field was typed as raw ByteList512KiB and producers manually called encode_bytes before storing the result; verify and post-block split paths then decode_bytes-ed it back. Promoting the field to its real type lets SSZ handle (de)serialization at the container boundary.
  2. 76ac8481 — Inline single-use type_two alias. Both verify_signatures and the sync post-block handler bound type_two = signed_block.proof and used the alias exactly once. Drop it.
  3. 7facb6d2 — Rename the multi-signature types to express what they are, not their Rust-prover convention.
    • Classes: TypeOneMultiSignatureSingleMessageAggregate, TypeTwoMultiSignatureMultiMessageAggregate.
    • Variables, test function names, and prose mentions follow.
    • External Rust binding names from leanMultisig-py (aggregate_type_1, verify_type_1, merge_many_type_1, split_type_2_by_msg, verify_type_2_with_messages) are unchanged. Snappy's unrelated "Copy Type 1/2" in node/snappy/encoding.py is also untouched.

Notes

  • The on-wire SSZ encoding of SignedBlock is unchanged — Container { proof: ByteList512KiB } and a bare ByteList512KiB produce the same [4-byte offset = 4][bytes] layout under a parent container's offset region.
  • hash_tree_root(SignedBlock) will change because Container merkleization differs from List[byte] merkleization. Any consensus fixtures or downstream code that hardcode block roots will need regeneration.

Diff size

Commit Files Lines
Type the field 11 +33 / −53
Inline alias 2 +2 / −5
Rename 26 +265 / −236

Test plan

  • just check — lint, format, ty, codespell, mdformat all clean
  • Targeted pytest on the most-affected files (validator service, xmss aggregation, attestation target fork choice, reqresp client range) — 99 passed
  • Full unit-test suite on CI
  • Regenerate consensus fixtures (uv run fill --fork=lstar --clean -n auto) and confirm any block-root-dependent vectors update cleanly

🤖 Generated with Claude Code

tcoratger and others added 3 commits May 29, 2026 17:42
The field was typed as raw ByteList512KiB and producers manually called
encode_bytes before storing the result; verify_signatures and the
post-block split path then decode_bytes-ed it back. Promote the field
to its real type so SSZ handles the (de)serialization once at the
container boundary.

- Removes the encode-then-wrap dance in the validator service and test
  builders.
- Removes the try/decode_bytes/except blocks in verify_signatures and
  the sync service post-block handler.
- Updates SSZ round-trip tests, fork choice tests, and reqresp client
  helpers to construct the typed envelope directly.
- Deletes four tautological decode-smoke assertions in test_service.py
  now that the Pydantic-validated field guarantees the shape.

The on-wire SSZ encoding of SignedBlock is unchanged; hash_tree_root
will change because Container merkleization differs from List[byte]
merkleization, so consensus fixtures that hardcode block roots need
regeneration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both call sites used the local alias exactly once. Drop it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ggregate / MultiMessageAggregate

TypeOneMultiSignature and TypeTwoMultiSignature were named after the
underlying Rust prover convention, not what they semantically are.
Rename them to express the actual data: an aggregate over a single
message versus an aggregate over many distinct messages.

- Classes: TypeOneMultiSignature -> SingleMessageAggregate,
  TypeTwoMultiSignature -> MultiMessageAggregate
- Variables and locals follow the snake_case form:
  type_1 -> single_message_aggregate, type_2 -> multi_message_aggregate
  (plus the compound forms type1_wire, type2_wire, block_t1,
  proposer_type_1, etc.)
- Test function names match: test_type_one_* -> test_single_message_*,
  test_type_two_* -> test_multi_message_*
- Prose mentions of "Type-1" / "Type-2" in docstrings and comments
  become "single-message aggregate" / "multi-message aggregate"

External Rust binding names (aggregate_type_1, verify_type_1,
merge_many_type_1, split_type_2_by_msg, verify_type_2_with_messages)
come from the leanMultisig-py package and are left untouched.

The "Type 1" / "Type 2" mentions in node/snappy/encoding.py refer to
Snappy's Copy Type 1 / Copy Type 2 wire encodings and are unrelated.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tcoratger tcoratger changed the title refactor(forks/lstar): type SignedBlock.proof as TypeTwoMultiSignature refactor(forks/lstar): type SignedBlock.proof and rename multi-signature types May 29, 2026
@tcoratger tcoratger merged commit 356a516 into leanEthereum:main May 29, 2026
13 checks passed
tcoratger added a commit to unnawut/leanSpec that referenced this pull request May 30, 2026
…renames

Rebase onto main and apply the renames/cleanups landed since the branch
was opened:

- PR leanEthereum#799: TypeOneMultiSignature → SingleMessageAggregate, proof_type
  literal "type_1" → "single_message", file renames
  test_type_1_{valid,invalid}.py → test_single_message_{valid,invalid}.py.
- PR leanEthereum#800: validator_ids → validator_indices, with_validator_id →
  with_validator_index, vid/pubkey expansions.
- post-leanEthereum#788/leanEthereum#790/leanEthereum#796 imports: lean_spec.spec.forks / .spec.ssz /
  .spec.crypto.*.

Replace the stringly-typed tamper dict with a Pydantic discriminated
union (RebindToAlternateHeadRoot, IncrementEmittedSlot,
SwapParticipantPublicKey). The match dispatch on the typed variants
drops the two type: ignore[index] casts and lets the test sites read
tamper=SwapParticipantPublicKey(index=0, with_validator_index=1)
instead of a magic-string dict.

Inline the four single-call helpers (_apply_tamper plus three
_tamper_*) and the verification helper into make_fixture so the four
phases — generate / tamper / verify / publish — are visible in one
method.

Drop both model_copy(update=...) calls per leanEthereum#789: direct field
construction for the frozen AttestationData rebuild, direct field
assignment for the mutable fixture self-update.

Replace the internal dict[str, Any] bundle with named locals; the
bundle never escapes make_fixture, so the dict adds nothing.

Guard SwapParticipantPublicKey against silent no-op swaps where the
replacement key happens to equal the original. Broaden the verifier
exception catch to surface unexpected exception types as
"expected X got Y" instead of crashing the filler.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tcoratger added a commit that referenced this pull request May 30, 2026
…heck proof verification (#786)

* test(consensus): add VerifyProofsTest fixture and Type-1 valid vectors

Introduces a new consensus fixture format that emits self-contained
multi-signature verification vectors so cross-client implementations
can run their own Type-1 verifier and compare outcomes.

Three positive vectors land alongside the fixture: a single-validator
baseline, a four-validator all-participating case, and a
four-validator non-contiguous bitfield case ([1, 0, 1, 1]).

The fixture also surfaces the spec-layer binding between attestation
data and proof: clients recompute hash_tree_root(attestation_data)
and must match the emitted message field before running the verifier.

* test(consensus): add Type-1 verify_proofs rejection vectors

Adds four negative vectors exercising spec-layer bindings between
inputs and the multi-signature proof. Each vector uses a tamper
operation on the fixture to produce a structurally valid bundle that
must be rejected by a conformant verifier:

- wrong_message: proof bound to an alternate head root inside the
  attestation data
- wrong_slot: emitted slot field shifted while the proof binding
  stays on the original slot
- wrong_public_keys: one emitted pubkey replaced with another
  validator's
- aggregation_bits_length_mismatch: emitted bits truncated while
  the pubkey count stays unchanged

Vectors covering malformed or truncated proof bytes are intentionally
out of scope: leanSpec consumes the multi-signature primitive as a
black box and primitive integrity belongs to its own conformance
suite. Pubkey ordering is also not a binding to test: the aggregator
sorts participants internally, so the verifier is order-insensitive.

* test(consensus): align VerifyProofsTest with sibling fixture conventions

Brings the new fixture in line with the patterns the other consensus
test fixtures follow:

- Drop ``from __future__ import annotations`` (PR #759 removed it from
  Pydantic-defining files); quote the one self-reference instead.
- Replace the bespoke ``expect_valid: bool`` field with the inherited
  ``expect_exception`` field already used by SSZTest, NetworkingCodec,
  and VerifySignaturesTest. Rejection vectors now pin
  ``AggregationError`` and the framework serializes the class name to
  JSON.
- Switch the tamper dispatch in ``_apply_tamper`` from ``if/elif`` to
  ``match/case`` to follow the pattern in slot_clock and
  networking_codec.
- Expand the module-level docstring from one line to a short
  paragraph describing what the fixture covers.
- Normalize the ``public_keys`` default from ``[]`` to ``| None =
  None`` to match every other output field on the model.

* test(consensus): drop aggregation_bits_length_mismatch rejection vector

The check that fires here is the early-reject in the spec wrapper's
verify method (len(public_keys) != participants.count(True)), not a
consensus-critical binding. In real consensus the inconsistency cannot
arise because clients resolve public keys from the bitfield plus the
validator registry as one operation. A client that did pass a wrong
pubkey count would also be rejected by the underlying recursive
verifier on its internal pubkey-set commitment, so the wrapper check
is at best an early exit with a nicer error message.

The remaining three rejection vectors still exercise the meaningful
spec-layer bindings: message hash, slot, and pubkey set.

* refactor(consensus): tighten VerifyProofsTest and absorb post-rebase renames

Rebase onto main and apply the renames/cleanups landed since the branch
was opened:

- PR #799: TypeOneMultiSignature → SingleMessageAggregate, proof_type
  literal "type_1" → "single_message", file renames
  test_type_1_{valid,invalid}.py → test_single_message_{valid,invalid}.py.
- PR #800: validator_ids → validator_indices, with_validator_id →
  with_validator_index, vid/pubkey expansions.
- post-#788/#790/#796 imports: lean_spec.spec.forks / .spec.ssz /
  .spec.crypto.*.

Replace the stringly-typed tamper dict with a Pydantic discriminated
union (RebindToAlternateHeadRoot, IncrementEmittedSlot,
SwapParticipantPublicKey). The match dispatch on the typed variants
drops the two type: ignore[index] casts and lets the test sites read
tamper=SwapParticipantPublicKey(index=0, with_validator_index=1)
instead of a magic-string dict.

Inline the four single-call helpers (_apply_tamper plus three
_tamper_*) and the verification helper into make_fixture so the four
phases — generate / tamper / verify / publish — are visible in one
method.

Drop both model_copy(update=...) calls per #789: direct field
construction for the frozen AttestationData rebuild, direct field
assignment for the mutable fixture self-update.

Replace the internal dict[str, Any] bundle with named locals; the
bundle never escapes make_fixture, so the dict adds nothing.

Guard SwapParticipantPublicKey against silent no-op swaps where the
replacement key happens to equal the original. Broaden the verifier
exception catch to surface unexpected exception types as
"expected X got Y" instead of crashing the filler.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant